Repository navigation
firmware: add owner OTA research code (held) - #24379
Draft
Git-on-my-level wants to merge 1 commit into
Draft
Git-on-my-level wants to merge 1 commit into
Git-on-my-level wants to merge 1 commit into
Conversation
Moved out of the CV1 contributor checks so it can land under its own title. It isn't built, and it has known defects listed on the PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
32 issues found across 33 files
Confidence score: 1/5
cv1_copy_verifiedinsecure_store.ccan start a copy fromTRIALorVERIFIEDand erase the personal area before recovery, risking the active or pending image. Gate the operation on an allowed journal phase and preserve those images.boot_epoch.ccan erase an active application or confirmed-reference journal when given the wrongcv1_flashview, and the journal API insecure_services.hdoes not enforce a boot-only boundary. Require physically constrained, role-specific flash handles.secure_store.ccan write the journal and personal flash before checking Secure isolation, so a backend without ownership evidence may modify protected state. Check isolation before any writes.cv1_image_admitinsecure_image.cmay call a garbage release callback when the output descriptor is uninitialized. Only release descriptors known to have been initialized.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="omi/firmware/research/owner_ota/target_adapter/bus_epoch/CMakeLists.txt">
<violation number="1" location="omi/firmware/research/owner_ota/target_adapter/bus_epoch/CMakeLists.txt:7">
P2: Enabling `CV1_RESEARCH_COMPILE_BUS_EPOCH` produces an incomplete target: `boot_epoch.c` depends on the journal and storage implementations, but this source list adds only the bus files. Add the required adapter and shared codec/image sources, or make the option explicitly depend on the target-adapter source set, so the documented opt-in build can link.</violation>
</file>
<file name="omi/firmware/research/owner_ota/shared/psa_adapter.c">
<violation number="1" location="omi/firmware/research/owner_ota/shared/psa_adapter.c:40">
P2: `begin`, `update`, `finish`, and `abort_hash` cast `struct cv1_digest` storage to `psa_hash_operation_t *`, but `struct cv1_digest` is declared as a `uint64_t` array rather than a PSA operation object. Store a real PSA operation or use a standards-compliant wrapper/copy instead of relying on an incompatible typed access.</violation>
</file>
<file name="omi/firmware/research/owner_ota/target_adapter/bus_epoch/README.md">
<violation number="1" location="omi/firmware/research/owner_ota/target_adapter/bus_epoch/README.md:92">
P2: This claims NOR/SD contention coverage, but the test only contends host callback fakes and does not exercise either Zephyr driver. Describe it as host-callback arbitration coverage so the verification section does not imply driver-level ownership was tested.</violation>
</file>
<file name="omi/firmware/docs/ota-recovery-link-research.md">
<violation number="1" location="omi/firmware/docs/ota-recovery-link-research.md:3">
P2: This status overstates the current verification state: the PR was not re-verified on this branch, yet the note says it was independently reviewed. Mark the result as external, unverified evidence so readers do not treat stale receipts as current validation.</violation>
<violation number="2" location="omi/firmware/docs/ota-recovery-link-research.md:26">
P2: The first capacity calculation is off by 12 bytes: 245252 B minus 136160 B equals 109092 B, not 109080 B. Correct the arithmetic or identify the omitted 12 bytes before using this budget as review evidence.</violation>
<violation number="3" location="omi/firmware/docs/ota-recovery-link-research.md:153">
P2: The late “actual PSA” section presents 39 cases, high-S coverage, and first NS-module integration as established, but this checkout contains neither the receipt nor a reproducible target/NS build for those claims. Check in the referenced evidence/build or label these results as unverified external claims so readers do not mistake the host stub for target integration.</violation>
</file>
<file name="omi/firmware/research/owner_ota/host/demo.c">
<violation number="1" location="omi/firmware/research/owner_ota/host/demo.c:280">
P1: This can erase the currently booted confirmed artifact after a reference-journal write failure. Protect the bank identified by the active trial `CV1_CONFIRMED` record as well as the reference bank, and reject the upload when no non-active bank remains.</violation>
</file>
<file name="omi/firmware/research/owner_ota/target_adapter/README.md">
<violation number="1" location="omi/firmware/research/owner_ota/target_adapter/README.md:69">
P2: This test claim overstates coverage: the fixture flips `ready` inside `io`, so it only exercises ownership loss after the I/O starts. Add a pre-I/O loss case or narrow the claim.</violation>
<violation number="2" location="omi/firmware/research/owner_ota/target_adapter/README.md:72">
P2: This ARM verification is not reproducible from the repository because the target adapter is not connected to any firmware build or test entry point. Add an explicit research build entry point, or label this as external and unverified evidence.</violation>
</file>
<file name="omi/firmware/research/owner_ota/host/fixtures.py">
<violation number="1" location="omi/firmware/research/owner_ota/host/fixtures.py:25">
P2: This filter silently drops PT_LOAD bytes that overlap the payload start from below, so an accepted `--payload-elf` can produce a fixture that differs from the supplied image. Reject overlapping segments (or copy their in-range intersection) before accepting the payload.</violation>
</file>
<file name="omi/firmware/research/owner_ota/target_adapter/CMakeLists.txt">
<violation number="1" location="omi/firmware/research/owner_ota/target_adapter/CMakeLists.txt:7">
P2: `test_adapter.c` is never wired into a build or test target, so the README's host-contract coverage is not exercised automatically. Add a host test executable and invoke it from the documented research test workflow.</violation>
</file>
<file name="omi/firmware/research/owner_ota/README.md">
<violation number="1" location="omi/firmware/research/owner_ota/README.md:7">
P3: The runner checks the terminal C event, then writes `report.json`; it never checks the report itself. Describe this as checking the terminal result or add validation of the generated report.</violation>
<violation number="2" location="omi/firmware/research/owner_ota/README.md:20">
P2: This command does not work on hosts whose C11/UBSan compiler is not `/usr/bin/cc`, because `run.py` uses that hard-coded default and the README provides no `--cc` override. Pass the compiler path explicitly or make the runner resolve `cc` from `PATH`.</violation>
<violation number="3" location="omi/firmware/research/owner_ota/README.md:39">
P2: This setup snippet cannot be followed as written because `CV1_NCS_ROOT` and `CV1_NATIVE_BUILD` are undefined; the command therefore fails before producing the required archive. Define both variables in the snippet or use the concrete paths from the preceding command.</violation>
<violation number="4" location="omi/firmware/research/owner_ota/README.md:68">
P2: The demo changes the personal firmware container from its initial bytes to A/B, so “all simulated user bytes are unchanged” is false. Limit this sentence to user-data and guard ranges.</violation>
<violation number="5" location="omi/firmware/research/owner_ota/README.md:111">
P3: The evidence instructions omit `source-inputs.json` and `fixtures.json`, which contain the exact source, ELF, and per-fixture hashes. Name those generated files so the documented evidence can actually be inspected.</violation>
</file>
<file name="omi/firmware/research/owner_ota/host/test_workflow.py">
<violation number="1" location="omi/firmware/research/owner_ota/host/test_workflow.py:20">
P2: The acceptance test does not require the documented FAULT-confirmation rejection scenario, so removing that safety case from the demo would still produce a passing test. Add `unhealthy_confirmation_rejected` to the required stages.</violation>
</file>
<file name="omi/firmware/research/owner_ota/target_adapter/test_adapter.c">
<violation number="1" location="omi/firmware/research/owner_ota/target_adapter/test_adapter.c:61">
P2: The ownership-loss test covers only loss after the I/O callback; it never calls `cv1_storage_read` while ownership is already unavailable. Add a read case with `f.ready = 0` and verify that no callback runs and the destination is invalidated.</violation>
</file>
<file name="omi/firmware/research/owner_ota/target_adapter/bus_epoch/boot_epoch.c">
<violation number="1" location="omi/firmware/research/owner_ota/target_adapter/bus_epoch/boot_epoch.c:25">
P1: This reservation can erase and rewrite an active application or confirmed-reference journal when the caller supplies the wrong `cv1_flash` view; the role marker does not prevent that alias. Require a dedicated, physically bound boot-ledger capability/backend and isolation check before allowing either reservation write.</violation>
</file>
<file name="omi/firmware/research/owner_ota/target_adapter/bus_epoch/test_bus_epoch.c">
<violation number="1" location="omi/firmware/research/owner_ota/target_adapter/bus_epoch/test_bus_epoch.c:21">
P2: `own` always reports ownership, so this suite never exercises ownership loss before or after I/O. Make ownership mutable and cover both transitions; otherwise regressions in either check still print PASS.</violation>
<violation number="2" location="omi/firmware/research/owner_ota/target_adapter/bus_epoch/test_bus_epoch.c:124">
P2: This regression executable is not wired into the repository build or test targets, so normal builds cannot run it and the checks can silently rot. Add an explicit native test target and invoke it from the research test workflow.</violation>
<violation number="3" location="omi/firmware/research/owner_ota/target_adapter/bus_epoch/test_bus_epoch.c:131">
P2: Successful-read checks never validate the returned bytes, so a missing or short bounce-to-destination copy can pass this suite. Compare every requested output buffer with the fake `0x5a` data for the direct and worker reads.</violation>
</file>
<file name="omi/firmware/research/owner_ota/target_adapter/bus_epoch/zephyr_bus.c">
<violation number="1" location="omi/firmware/research/owner_ota/target_adapter/bus_epoch/zephyr_bus.c:15">
P2: These assertions validate only pin number 11, not the GPIO controller, so they do not enforce the claimed P0.11/P1.11 binding. A DTS swap between `gpio0` and `gpio1` would still compile while `finish()` observes the wrong CS; assert each controller identity as well.</violation>
</file>
<file name="omi/firmware/research/owner_ota/shared/secure_image.c">
<violation number="1" location="omi/firmware/research/owner_ota/shared/secure_image.c:18">
P2: `cv1_image_forget` erases the lease and release callback even when `source.release` fails. The caller receives `CV1_EIO` but can no longer retry release, potentially stranding the source bank; preserve the descriptor until release succeeds.</violation>
<violation number="2" location="omi/firmware/research/owner_ota/shared/secure_image.c:125">
P1: `cv1_image_admit` calls `cv1_image_forget(out)` before establishing that `out` contains a valid initialized descriptor. A first admission into an uninitialized image can invoke a garbage release callback; add an explicit initialization lifecycle or make admission safely initialize fresh outputs while preserving safe re-admission.</violation>
</file>
<file name="omi/firmware/research/owner_ota/shared/secure_services.h">
<violation number="1" location="omi/firmware/research/owner_ota/shared/secure_services.h:78">
P1: The journal API cannot enforce the boot-only boundary: any trusted caller can map `CV1_JOURNAL` to the trial, confirmed-reference, personal, or another flash view. Split boot-ledger and application/reference handles or make the storage handles opaque and fixed to their physical regions before adding a gateway.</violation>
<violation number="2" location="omi/firmware/research/owner_ota/shared/secure_services.h:94">
P1: This token has no boot/reset epoch, so a prepared token can be replayed after a warm reset and bypass the documented fresh-admission requirement. Bind it to the boot authority’s fresh epoch and reject tokens from prior epochs before changing the journal or issuing a boot token.</violation>
</file>
<file name="omi/firmware/research/owner_ota/shared/secure_store.c">
<violation number="1" location="omi/firmware/research/owner_ota/shared/secure_store.c:28">
P1: `ops` allows journal and personal-flash writes without checking the mandatory Secure isolation callback. `cv1_authorize_boot` checks isolation only after these writes, so a backend without ownership evidence can already modify protected regions; require isolation before every write path.</violation>
<violation number="2" location="omi/firmware/research/owner_ota/shared/secure_store.c:201">
P0: `cv1_copy_verified` starts a new copy from every valid journal phase, including `TRIAL` and `VERIFIED`, then erases the personal area. A caller can overwrite the active or pending image before recovery; separate normal-copy and confirmed-reference recovery paths, and reject candidate copies during those phases.</violation>
<violation number="3" location="omi/firmware/research/owner_ota/shared/secure_store.c:306">
P1: This authorization step can issue a boot token without revalidating the destination bytes after the preparation transition. Make the final attribution/ownership operation atomically exclude writers and authenticate the destination readback before returning `CV1_TOKEN`; a point-in-time `isolation_ready` result is insufficient.</violation>
</file>
<file name="omi/firmware/research/owner_ota/host/run.py">
<violation number="1" location="omi/firmware/research/owner_ota/host/run.py:43">
P3: The report says `artifact_admission` is false even though the host run performs shared-C artifact admission. Rename this to a target-only field or report host admission separately so consumers cannot misread the evidence.</violation>
</file>
<file name="omi/firmware/research/owner_ota/target_adapter/storage_adapter.c">
<violation number="1" location="omi/firmware/research/owner_ota/target_adapter/storage_adapter.c:80">
P2: `cv1_storage_read` copies successful chunks before the complete request succeeds, then returns `-1` without invalidating earlier or pre-existing destination bytes. Clear the entire destination on every failure or stage output until all chunks succeed.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| if (rc == CV1_EIO) | ||
| return rc; | ||
| memset(&r, 0, sizeof(r)); | ||
| r.sequence = rc ? 1 : old.sequence + 1; |
There was a problem hiding this comment.
P0: cv1_copy_verified starts a new copy from every valid journal phase, including TRIAL and VERIFIED, then erases the personal area. A caller can overwrite the active or pending image before recovery; separate normal-copy and confirmed-reference recovery paths, and reject candidate copies during those phases.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At omi/firmware/research/owner_ota/shared/secure_store.c, line 201:
<comment>`cv1_copy_verified` starts a new copy from every valid journal phase, including `TRIAL` and `VERIFIED`, then erases the personal area. A caller can overwrite the active or pending image before recovery; separate normal-copy and confirmed-reference recovery paths, and reject candidate copies during those phases.</comment>
<file context>
@@ -0,0 +1,324 @@
+ if (rc == CV1_EIO)
+ return rc;
+ memset(&r, 0, sizeof(r));
+ r.sequence = rc ? 1 : old.sequence + 1;
+ if (!rc && old.sequence == UINT64_MAX)
+ return CV1_ESTATE;
</file context>
| return -1; | ||
| if (known < 0 && !d.initializing) | ||
| return -1; | ||
| int slot = known < 0 ? 0 : 1 - known; |
There was a problem hiding this comment.
P1: This can erase the currently booted confirmed artifact after a reference-journal write failure. Protect the bank identified by the active trial CV1_CONFIRMED record as well as the reference bank, and reject the upload when no non-active bank remains.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At omi/firmware/research/owner_ota/host/demo.c, line 280:
<comment>This can erase the currently booted confirmed artifact after a reference-journal write failure. Protect the bank identified by the active trial `CV1_CONFIRMED` record as well as the reference bank, and reject the upload when no non-active bank remains.</comment>
<file context>
@@ -0,0 +1,666 @@
+ return -1;
+ if (known < 0 && !d.initializing)
+ return -1;
+ int slot = known < 0 ? 0 : 1 - known;
+ if (d.bank[slot].leased)
+ return -1;
</file context>
| * above the last issued epoch. Never reseed a corrupt/erased ledger. */ | ||
| for (unsigned step = 0; step < 2; step++) { | ||
| next.sequence++; | ||
| if (cv1_journal_store(f, &next) || cv1_journal_load(f, &readback) || readback.sequence != next.sequence || |
There was a problem hiding this comment.
P1: This reservation can erase and rewrite an active application or confirmed-reference journal when the caller supplies the wrong cv1_flash view; the role marker does not prevent that alias. Require a dedicated, physically bound boot-ledger capability/backend and isolation check before allowing either reservation write.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At omi/firmware/research/owner_ota/target_adapter/bus_epoch/boot_epoch.c, line 25:
<comment>This reservation can erase and rewrite an active application or confirmed-reference journal when the caller supplies the wrong `cv1_flash` view; the role marker does not prevent that alias. Require a dedicated, physically bound boot-ledger capability/backend and isolation check before allowing either reservation write.</comment>
<file context>
@@ -0,0 +1,60 @@
+ * above the last issued epoch. Never reseed a corrupt/erased ledger. */
+ for (unsigned step = 0; step < 2; step++) {
+ next.sequence++;
+ if (cv1_journal_store(f, &next) || cv1_journal_load(f, &readback) || readback.sequence != next.sequence ||
+ readback.generation != CV1_EPOCH_ROLE || readback.phase != CV1_CONFIRMED || readback.size != 1024 ||
+ memcmp(readback.hash, entropy, 32))
</file context>
| uint8_t hash[32]; | ||
| }; | ||
| struct cv1_boot_token { | ||
| uint32_t valid, msp, reset, payload_size, generation; |
There was a problem hiding this comment.
P1: This token has no boot/reset epoch, so a prepared token can be replayed after a warm reset and bypass the documented fresh-admission requirement. Bind it to the boot authority’s fresh epoch and reject tokens from prior epochs before changing the journal or issuing a boot token.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At omi/firmware/research/owner_ota/shared/secure_services.h, line 94:
<comment>This token has no boot/reset epoch, so a prepared token can be replayed after a warm reset and bypass the documented fresh-admission requirement. Bind it to the boot authority’s fresh epoch and reject tokens from prior epochs before changing the journal or issuing a boot token.</comment>
<file context>
@@ -0,0 +1,107 @@
+ uint8_t hash[32];
+};
+struct cv1_boot_token {
+ uint32_t valid, msp, reset, payload_size, generation;
+ uint64_t journal_sequence;
+ uint8_t hash[32];
</file context>
| * F0000..F2000 and personal to54000..F0000. Host demo reuses ONLY the journal | ||
| * codec through a separately pinned reference view F2000..F4000; it rejects | ||
| * personal access on that view. No real target backend or generic erase API. */ | ||
| struct cv1_flash { |
There was a problem hiding this comment.
P1: The journal API cannot enforce the boot-only boundary: any trusted caller can map CV1_JOURNAL to the trial, confirmed-reference, personal, or another flash view. Split boot-ledger and application/reference handles or make the storage handles opaque and fixed to their physical regions before adding a gateway.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At omi/firmware/research/owner_ota/shared/secure_services.h, line 78:
<comment>The journal API cannot enforce the boot-only boundary: any trusted caller can map `CV1_JOURNAL` to the trial, confirmed-reference, personal, or another flash view. Split boot-ledger and application/reference handles or make the storage handles opaque and fixed to their physical regions before adding a gateway.</comment>
<file context>
@@ -0,0 +1,107 @@
+ * F0000..F2000 and personal to54000..F0000. Host demo reuses ONLY the journal
+ * codec through a separately pinned reference view F2000..F4000; it rejects
+ * personal access on that view. No real target backend or generic erase API. */
+struct cv1_flash {
+ void *context;
+ int (*read)(void *, enum cv1_area, uint32_t, uint8_t *, size_t);
</file context>
| size_t done = 0; | ||
| while (done < n) { | ||
| size_t chunk = n - done > CV1_BOUNCE ? CV1_BOUNCE : n - done; | ||
| if (!owned(s) || s->ops.io(s->ops.context, 0, s->base + o + (uint32_t) done, s->bounce, chunk) != (int) chunk || |
There was a problem hiding this comment.
P2: cv1_storage_read copies successful chunks before the complete request succeeds, then returns -1 without invalidating earlier or pre-existing destination bytes. Clear the entire destination on every failure or stage output until all chunks succeed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At omi/firmware/research/owner_ota/target_adapter/storage_adapter.c, line 80:
<comment>`cv1_storage_read` copies successful chunks before the complete request succeeds, then returns `-1` without invalidating earlier or pre-existing destination bytes. Clear the entire destination on every failure or stage output until all chunks succeed.</comment>
<file context>
@@ -0,0 +1,109 @@
+ size_t done = 0;
+ while (done < n) {
+ size_t chunk = n - done > CV1_BOUNCE ? CV1_BOUNCE : n - done;
+ if (!owned(s) || s->ops.io(s->ops.context, 0, s->base + o + (uint32_t) done, s->bounce, chunk) != (int) chunk ||
+ !owned(s)) {
+ s->poisoned = 1;
</file context>
| @@ -0,0 +1,187 @@ | |||
| # CV1 key-free recovery link research | |||
|
|
|||
| Status: isolated static research, independently reviewed; not deployable firmware. | |||
There was a problem hiding this comment.
P2: This status overstates the current verification state: the PR was not re-verified on this branch, yet the note says it was independently reviewed. Mark the result as external, unverified evidence so readers do not treat stale receipts as current validation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At omi/firmware/docs/ota-recovery-link-research.md, line 3:
<comment>This status overstates the current verification state: the PR was not re-verified on this branch, yet the note says it was independently reviewed. Mark the result as external, unverified evidence so readers do not treat stale receipts as current validation.</comment>
<file context>
@@ -0,0 +1,187 @@
+# CV1 key-free recovery link research
+
+Status: isolated static research, independently reviewed; not deployable firmware.
+Legacy OTA bootstrap and maximum-version policy remain held. No target, key,
+signing, provisioning, bootloader or protection change occurred.
</file context>
Suggested change
| Status: isolated static research, independently reviewed; not deployable firmware. | |
| Status: isolated static research; not re-verified on this branch and not deployable firmware. |
| No real supervisor/release signing. Research/demo artifacts are excluded from | ||
| admission; no workflow can sign/publish/deploy through this program. | ||
|
|
||
| Inspect report.json and 03-host-demo.json: C exit/stdout/stderr, event-level artifact |
There was a problem hiding this comment.
P3: The evidence instructions omit source-inputs.json and fixtures.json, which contain the exact source, ELF, and per-fixture hashes. Name those generated files so the documented evidence can actually be inspected.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At omi/firmware/research/owner_ota/README.md, line 111:
<comment>The evidence instructions omit `source-inputs.json` and `fixtures.json`, which contain the exact source, ELF, and per-fixture hashes. Name those generated files so the documented evidence can actually be inspected.</comment>
<file context>
@@ -0,0 +1,144 @@
+No real supervisor/release signing. Research/demo artifacts are excluded from
+admission; no workflow can sign/publish/deploy through this program.
+
+Inspect report.json and 03-host-demo.json: C exit/stdout/stderr, event-level artifact
+hashes, exact source inputs, ELF/fixture/native archive/executable hashes, and explicit
+simulation labels. Deterministic model decisions do not promise bit-identical
</file context>
Suggested change
| Inspect report.json and 03-host-demo.json: C exit/stdout/stderr, event-level artifact | |
| +Inspect report.json, source-inputs.json, fixtures.json, and 03-host-demo.json: C exit/stdout/stderr, event-level artifact |
| It compiles the actual shared C streaming verifier, PSA adapter, journal, copy and | ||
| boot-authorization services. A single C driver supplies in-memory NOR, a 137-byte | ||
| transport stub, fixture-owner ACKs and explicit HEALTHY/FAULT boot outcomes. | ||
| Python only builds/signs public test fixtures, launches C and checks its report; |
There was a problem hiding this comment.
P3: The runner checks the terminal C event, then writes report.json; it never checks the report itself. Describe this as checking the terminal result or add validation of the generated report.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At omi/firmware/research/owner_ota/README.md, line 7:
<comment>The runner checks the terminal C event, then writes `report.json`; it never checks the report itself. Describe this as checking the terminal result or add validation of the generated report.</comment>
<file context>
@@ -0,0 +1,144 @@
+It compiles the actual shared C streaming verifier, PSA adapter, journal, copy and
+boot-authorization services. A single C driver supplies in-memory NOR, a 137-byte
+transport stub, fixture-owner ACKs and explicit HEALTHY/FAULT boot outcomes.
+Python only builds/signs public test fixtures, launches C and checks its report;
+it does not implement the OTA policy. No serial/debugger/device path or service.
+
</file context>
Suggested change
| Python only builds/signs public test fixtures, launches C and checks its report; | |
| Python only builds/signs public test fixtures, launches C and checks its terminal result; |
| if not events or events[-1]['stage']!='PASS':raise RuntimeError('no terminal C PASS') | ||
| current={str(x.relative_to(root)):hashlib.sha256(x.read_bytes()).hexdigest() for x in source_paths} | ||
| if current!=inputs:raise RuntimeError('source changed during demo') | ||
| report={'schema':1,'passed':True,'layer':'HOST transport/boot/flash simulation with actual sharedC+PSA','transport':'137-byte ordered C stub; notBLE','boot':'explicit outcome stub; notCPUemulation','owner_registry':'public test fixture input only; not enrolled/persistentcredentials','target_operations':False,'production_signing':False,'test_fixture_signing':True,'release_authorized':False,'artifact_admission':False,'source_sha256':hashlib.sha256((out/'source-inputs.json').read_bytes()).hexdigest(),'executable_sha256':hashlib.sha256(executable.read_bytes()).hexdigest(),'native_mbedcrypto_sha256':hashlib.sha256(a.mbedcrypto.read_bytes()).hexdigest(),'fixtures_manifest_sha256':hashlib.sha256((out/'fixtures.json').read_bytes()).hexdigest(),'events':events} |
There was a problem hiding this comment.
P3: The report says artifact_admission is false even though the host run performs shared-C artifact admission. Rename this to a target-only field or report host admission separately so consumers cannot misread the evidence.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At omi/firmware/research/owner_ota/host/run.py, line 43:
<comment>The report says `artifact_admission` is false even though the host run performs shared-C artifact admission. Rename this to a target-only field or report host admission separately so consumers cannot misread the evidence.</comment>
<file context>
@@ -0,0 +1,48 @@
+ if not events or events[-1]['stage']!='PASS':raise RuntimeError('no terminal C PASS')
+ current={str(x.relative_to(root)):hashlib.sha256(x.read_bytes()).hexdigest() for x in source_paths}
+ if current!=inputs:raise RuntimeError('source changed during demo')
+ report={'schema':1,'passed':True,'layer':'HOST transport/boot/flash simulation with actual sharedC+PSA','transport':'137-byte ordered C stub; notBLE','boot':'explicit outcome stub; notCPUemulation','owner_registry':'public test fixture input only; not enrolled/persistentcredentials','target_operations':False,'production_signing':False,'test_fixture_signing':True,'release_authorized':False,'artifact_admission':False,'source_sha256':hashlib.sha256((out/'source-inputs.json').read_bytes()).hexdigest(),'executable_sha256':hashlib.sha256(executable.read_bytes()).hexdigest(),'native_mbedcrypto_sha256':hashlib.sha256(a.mbedcrypto.read_bytes()).hexdigest(),'fixtures_manifest_sha256':hashlib.sha256((out/'fixtures.json').read_bytes()).hexdigest(),'events':events}
+ (out/'report.json').write_text(json.dumps(report,indent=2)+'\n')
+ print(json.dumps({'passed':True,'report':str(out/'report.json'),'events':len(events),'target_operations':False}))
</file context>
Git-on-my-level
marked this pull request as draft
November 4, 2026 11:36
Member
Author
|
held by owning lane, do not merge |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by David
What changed and why
Adds the owner-OTA research code and the recovery-link research note, moved out of #24354. Everyday users must eventually be able to install personal firmware without a jig, and this code explores that path. It existed only in a scratch worktree until now.
Held, do not merge. It has known defects, listed below. Nothing builds it or ships it, and it changes no firmware behavior.
Product invariants affected
None. Nothing under
omi/firmware/research/is compiled into firmware.How it was verified
Not re-verified on this branch. It only moves these files onto current
main, unchanged from the commits reviewed on #24354. Earlier host-demo results are not claimed here. Nothing was run on hardware, and no release claim is made.Open findings to fix before this can leave draft
Reported by Cubic on #24354 against this code and not yet fixed or checked:
research/owner_ota/target_adapter/bus_epoch/boot_epoch.h:10(P1): This API does not enforce the boot-only ledger boundary it promises: any trusted caller can pass an application or referencecv1_flash, and the reservation then mutates that journal.research/owner_ota/host/demo.c:280(P1): This can erase the currently active confirmed artifact after an interrupted reference promotion.research/owner_ota/shared/secure_image.c:125(P1):cv1_image_admitcallscv1_image_forgetonoutbefore establishing thatoutis initialized, so first-use admission can call an indeterminate release callback.research/owner_ota/target_adapter/bus_epoch/CMakeLists.txt:7(P2): EnablingCV1_RESEARCH_COMPILE_BUS_EPOCHalone cannot link because this target omits its adapter and journal dependencies.research/owner_ota/target_adapter/bus_epoch/README.md:111(P2): This claims successful ARM compile/link verification that was not performed for this PR.research/owner_ota/host/fixtures.py:27(P2): The placement check reserves no space for the appended variant bytes, so a payload at the allowed maximum produces over-cap B/C fixtures and makes the host workflow fail.research/owner_ota/host/test_workflow.py:20(P2):requiredomitsunhealthy_confirmation_rejected, the report stage for the simulated FAULT confirmation path.research/owner_ota/target_adapter/storage_adapter.c:84(P2): A failed multi-chunk read leaves previously copied bytes indst, violating the adapter’s all-destination-invalidation contract and allowing stale/partial data to remain observable after an error.research/owner_ota/shared/psa_adapter.c:40(P2): This adapter stores a PSA operation in an object that is not declared as apsa_hash_operation_t, so the casts invoke undefined behavior under strict-aliasing builds and can become invalid when the PSA implementation changes its alignment or representation.research/owner_ota/target_adapter/zephyr_storage.c:1(P2): This source is never compiled by any repository build, so its claimed NCS/Zephyr compile checks and static assertions are not exercised.research/owner_ota/target_adapter/test_adapter.c:1(P2): This host contract test is not wired into any build or test runner, so its assertions never execute in the repository’s documented path.research/owner_ota/target_adapter/test_adapter.c:61(P2): The ownership-loss coverage only tests loss after the I/O callback.research/owner_ota/target_adapter/test_adapter.c:55(P2): This partial-I/O check does not verify the documented destination-invalidation contract.research/owner_ota/host/run.py:26(P2): A relative--ccpath fails becausecommandchanges to the generated output directory before the compiler path is resolved.research/owner_ota/shared/secure_image.c:18(P2):cv1_image_forgetdestroys the release callback, context, and lease after a failed release, making a still-held source lease unreleasable.research/owner_ota/target_adapter/bus_epoch/zephyr_bus.c:36(P2): The SD branch uses the wrong disk-access identifier for the OMI board, so every sector-zero request will fail under the normalSDMMCregistration.research/owner_ota/shared/secure_store.c:210(P2): This copy leaves stale bytes from a previously larger personal image whenever the new image is smaller.research/owner_ota/target_adapter/bus_epoch/secure_bus.c:74(P2): An oldcv1_bus_nor_iocallback can regain access after bus reinitialization because it reads the new mutableb->epoch.docs/ota-recovery-link-research.md:26(P3): The loaded-byte arithmetic is off by 12 bytes: 245252B minus 136160B is 109092B, not 109080B.research/owner_ota/target_adapter/bus_epoch/zephyr_bus.c:14(P3): These assertions do not actually enforce the P0.11/P1.11 binding stated in their messages.Tests
None added here. Two of the findings above are that the existing tests aren't wired into any runner.
UI evidence
UI-Evidence: none -- firmware research code and docs only
🤖 Generated with Claude Code